Skip to content

Add waitForAllTaskTermination for fixing segmentation fault of Carthage#24

Merged
robrix merged 1 commit intoCarthage:masterfrom
norio-nomura:wait-for-all-task-termination
Jun 5, 2015
Merged

Add waitForAllTaskTermination for fixing segmentation fault of Carthage#24
robrix merged 1 commit intoCarthage:masterfrom
norio-nomura:wait-for-all-task-termination

Conversation

@norio-nomura
Copy link
Copy Markdown
Contributor

@norio-nomura norio-nomura commented Jun 3, 2015

This is rebased version of #21.

Fix segmentation fault on termination of Carthage.

Segmentation fault on termination of Carthage 0.7.4 beta-2 happens following scenario:

  1. launchTask's thread call sendCompleted() and kick main thread
    2a. main thread wake up and call exit()
    2b. launchTask's thread continue disposing
  2. main thread cause segmentation fault.

Waiting completed from launchTask's thread is not enough for fixing the segmentation fault.
It needs waiting actual termination of launchTask's thread

Carthage/Carthage#474 (comment)

@norio-nomura
Add waitForAllTaskTermination
f141b0e 
Fix segmentation fault on Carthage termination.
Carthage/Carthage#474 (comment)
This was referenced Jun 3, 2015
Merged
Merged
@robrix
Copy link
Copy Markdown

robrix commented Jun 4, 2015

It looks like @jspahrsummers left some feedback on #21. Could you that feedback on this PR please?

@norio-nomura
Copy link
Copy Markdown
Contributor Author

norio-nomura commented Jun 4, 2015

Thanks for reviewing.
As @jspahrsummers wrote on #21 (comment), his comments on lines are not valuable for the purpose of this PR.
He implemented #23 from my suggestion by #21, using fine-grained and waiting per-task logic using RAC way. But #23 did not be replacement of #21.
So, I push this PR.

@robrix
Copy link
Copy Markdown

robrix commented Jun 5, 2015

Okay, thanks for the info. I think given the practical considerations that we ought to merge this PR, and then if it needs adjustment we can do that later on.

Thanks very much for the PR and your investigation into the segfaults, @norio-nomura!

robrix pushed a commit that referenced this pull request Jun 5, 2015
Merge pull request #24 from norio-nomura/wait-for-all-task-termination
e76c738 
Add waitForAllTaskTermination for fixing segmentation fault of Carthage
@robrix robrix merged commit e76c738 into Carthage:master Jun 5, 2015
@norio-nomura
Copy link
Copy Markdown
Contributor Author

norio-nomura commented Jun 5, 2015

Thank you for reviewing & merging it.

I have one more ReactiveCocoa/ReactiveCocoa#2058 related to Carthage hangs. I wish it will be also merged to next release.

@robrix
Copy link
Copy Markdown

robrix commented Jun 5, 2015

Cool, merged.

@norio-nomura
Copy link
Copy Markdown
Contributor Author

norio-nomura commented Jun 5, 2015

Thanks!

@norio-nomura
Copy link
Copy Markdown
Contributor Author

norio-nomura commented Jun 5, 2015

Oh, I should said about ReactiveCocoa before I pushed update Cartfile, Sorry. 😔

@robrix
Copy link
Copy Markdown

robrix commented Jun 5, 2015

Don’t sweat it!

@norio-nomura
Copy link
Copy Markdown
Contributor Author

norio-nomura commented Jun 5, 2015

Thanks,
I don't have any pending PRs about Carthage.

@norio-nomura norio-nomura deleted the wait-for-all-task-termination branch June 6, 2015 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@norio-nomura @robrix